-
Notifications
You must be signed in to change notification settings - Fork 10
adding secret watcher to restart ptp4l process #111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Thanks for your PR, |
pkg/daemon/daemon.go
Outdated
| chronydProcessName, // there can be only one chronyd process in the system | ||
| } | ||
|
|
||
| // saFileInfo tracks authentication file information for a profile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What specifically is sa short for in this context?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
security association, sa_file is an option inside the ptp4lconf's global section where we can add a filepath to mount a specific secret inside the linuxptp-daemon-container.
basically I converted some of the code from the ptpconfig_controller here because the file watcher forces a restart on the pods in case a change is detected on the mounted secret, so instead we're having a restart on the ptp4l process.
pkg/daemon/daemon.go
Outdated
| } | ||
| } | ||
|
|
||
| return changed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps you could return the new hash that way you don't have to recompute them again in the update function?
98fdb45 to
a3fcf96
Compare
pkg/daemon/daemon.go
Outdated
| } | ||
| // Filter for relevant events (Write, Create - Kubernetes atomic updates) | ||
| // Ignore events on temporary/hidden files | ||
| if event.Op&(fsnotify.Write|fsnotify.Create) == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should delete also be handled as a change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you mean "deleting the sa_file" then the controller actually removes the volumeMount in case the user delete it from the ptp4lconf or manually from the file system of the container. which forces a restart on the daemonset.
| case err, ok := <-watcherErrors: | ||
| // fsnotify watcher error | ||
| if !ok { | ||
| watcherErrors = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should you recreate the watcher here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should keep recreate until watcher is not having error
pkg/daemon/daemon.go
Outdated
|
|
||
| // Setup fsnotify watch on directory (not file, due to Kubernetes symlinks) | ||
| if dn.saFileWatcher != nil { | ||
| dirPath := filepath.Dir(saFilePath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be location where all sa_files are mounted. This path should controlled by the ptp-operator so is known ahead of time.
Instead of relying on the ptpconfig it should just recursively watch all dirs within that path.
That way theirs no need to have this late binding approach it can just be there from the start.
| } | ||
| glog.Info("PtpConfig change detected, restarting PTP processes") | ||
| dn.restartInProgress.Store(true) | ||
| err := dn.applyNodePTPProfiles() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are not run as goroutines so they are blocking meaning there is no need for this flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right, there is no concurrent access.. I will fix that
16ed150 to
915d95f
Compare
…ction - Add fsnotify watcher to monitor /etc/ptp-secret-mount/ for instant detection of security file changes (sa_file) - Trigger PTP process restart on sa_file write/create/remove events - Add extractAuthSettingsForPhc2sys() to parse auth settings from ptp4lConf - Inject auth settings into phc2sys config for grandmaster profiles - Promotes fsnotify from indirect to direct dependency
f594bd0 to
258876f
Compare
Add fsnotify-based sa_file Change Detection and phc2sys Auth Injection
Summary
Implement instant detection of PTP authentication file (sa_file) changes using fsnotify to enable ptp4l process restarts without pod restarts. Also adds automatic injection of authentication settings into phc2sys for grandmaster profiles.
Motivation
When PTP authentication secrets are updated in Kubernetes, the mounted sa_file content changes but linuxptp-daemon needed faster, event-driven detection instead of polling. Additionally, grandmaster profiles require phc2sys to use the same authentication settings as ptp4l, which wasn't being handled automatically.
Changes
New Components
saFileWatcher: fsnotify file system watcher for instant sa_file change detection
PtpSecFolder constant: Standardized mount path
/etc/ptp-secret-mount/Modified Functions
New(): Initializes fsnotify watcher to monitor
PtpSecFolderfrom daemon startupRun(): Added select cases for fsnotify events (Write/Create/Remove) and watcher error handling
applyNodePtpProfile(): Injects auth settings into phc2sys config for grandmaster profiles
New Functions
extractAuthSettingsForPhc2sys(): Parses ptp4lConf [global] section and extracts sa_file, spp, and active_key_id for phc2sys configurationBehavior
On daemon startup, watches
/etc/ptp-secret-mount/directory for file system eventsWhen sa_file changes (Write/Create/Remove), immediately triggers
applyNodePTPProfiles()to restart ptp4l processUses same restart mechanism as ConfigMap changes (no pod restart)
For grandmaster profiles, automatically injects auth settings from ptp4lConf into phc2sys
Ignores hidden files (like Kubernetes
..datasymlinks)Benefits
✅ Instant detection - event-driven vs polling-based approach
✅ No pod restarts for authentication secret updates
✅ Consistent with ConfigMap change handling
✅ Automatic phc2sys auth injection for grandmaster profiles
✅ Handles file Write, Create, and Remove events
✅ Lower resource usage (event-driven vs periodic polling)
Dependencies
github.com/fsnotify/fsnotifyfrom indirect to direct dependency.